Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Set lifecycle rules #9

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Set lifecycle rules #9

wants to merge 3 commits into from

Conversation

Lowercases
Copy link
Contributor

As a test for r101-api.

@Lowercases Lowercases force-pushed the test-lifecycle-rules-ecr branch from 6f654c6 to 63571d1 Compare May 26, 2022 18:26
@@ -32,6 +32,36 @@ workflows:
tag: "latest,$CIRCLE_BRANCH,$CIRCLE_SHA1"
# create the AWS ECR repo if it does not exist already.
create-repo: true
lifecycle-policy-path: >
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The documentation says this should be a path, not the actual content.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes! But it's a lie.

I started adding a file as in https://github.com/remind101/r101-api/pull/16623/files#diff-2c27a3138e2c83cb750d9a8a3b415799e8f3806ae6bd8a9d06a6abb23e018d36 (only left there for dev purposes) only to get Invalid length for parameter lifecyclePolicyText, value: 28, valid min length: 100 (https://app.circleci.com/pipelines/github/remind101/r101-api/11123/workflows/b3566098-95c6-4f13-9fd7-8a7f3f79a4d1/jobs/257499).

After my initial wtf moment, my suspects were confirmed when I tried the current approach and bingo! got An error occurred (AccessDeniedException) when calling the PutLifecyclePolicy operation: User: arn:aws:iam::************:user/erc-stage-circleci is not authorized to perform: ecr:PutLifecyclePolicy on resource: arn:aws:ecr:*********:************:repository/remind101/r101-api because no identity-based policy allows the ecr:PutLifecyclePolicy action (https://app.circleci.com/pipelines/github/remind101/r101-api/11188/workflows/00a85887-5e28-4912-81d6-94738f99dbe9/jobs/258057).

Trust no one.

Copy link
Contributor

@isobit isobit May 26, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

WTF lol. Well, at least it would be good to have a comment to explain that.

valid min length: 100

🤔

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What do you mean with "at least"?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

added comment

@Lowercases Lowercases requested a review from rleira May 26, 2022 19:06
.circleci/config.yml Outdated Show resolved Hide resolved
Avoid removing "master" and "main" tagged images to avoid removing a
running image after aggressive master/main rebuilding.

In order to glob all master and main images, add a composite tag for the
branch and build number.
# tags for this image, comma separated. A composite of branch+sha1 is
# added so the lifecycle rule (below) can filter images beginning
# with "master" or "main" to avoid applying aggressive rules.
tag: "latest,$CIRCLE_BRANCH,$CIRCLE_SHA1,$CIRCLE_BRANCH-$CIRCLE_BUILD_NUM"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think I'd still want to keep the commit SHA in the prefixed tag, because it's nice to have that info embedded and also would make it easier to transition to something other than CircleCI without causing conflicts.

Copy link

@rleira rleira left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great I think that having a space of 10k and making the master/main branches have a longer longevity (if I understood that part) covers our cases very well. Custom branches for custom deploys will probably be due well before 90 days

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants